I just realised thte sdmmc library in ESP-IDF v4.4 doesn't support erase! 😅

This is a hard one - I think I have to either backport a erase function or just disable SD card support for v4.4 target. I'd prefer the second option as our company and myself won't use IDF v4.4 in 2024 at all and I prefer not to provide support for this ESP-IDF version.

What do you think? @BrianPugh

I've converted this PR to a draft until you mark it as ready.

The read_size, prog_size and block_size in LittleFS configuration are forced to set to the same size of SD card sector.

I don't really see an issue with this, should I 😅 ?

This is a hard one - I think I have to either backport a erase function or just disable SD card support for v4.4 target. I'd prefer the second option as our company and myself won't use IDF v4.4 in 2024 at all and I prefer not to provide support for this ESP-IDF version.

I agree that disabling SD support in v4.4 is the way to go.

Hi @BrianPugh , thanks for your feedback!

In regards to disabling the SD card support in v4.4, I've tried adding this in to Kconfig:

    config LITTLEFS_SDMMC_SUPPORT
        bool "SDMMC support"
        default y
        depends on IDF_VERSION_MAJOR >= 5
        help
            Toggle SD card support
            This requires IDF v5+ as older ESP-IDF do not support SD card erase.

and soon I realise depends on IDF_VERSION_MAJOR >= 5 will not work, as in the context of Kconfig there are no IDF_VER or IDF_VERSON_* variables. 😅

I guess I have to do that in CMake...

Jackson

A slightly inelegant solution is to not have it depends on anything, and then raise a compilation warning in the code:

#if CONFIG_LITTLEFS_SDMMC_SUPPORT && IDF_VERSION_MAJOR  < 5
#error "SDMMC is not supported on esp-idf <5.0.0"
#endif

It doesn't have to be perfect since esp-idf 4.4 EOL is realtively soon (as you mentioned).

A slightly inelegant solution is to not have it depends on anything, and then raise a compilation warning in the code:

Yea sure, I'm doing this now.

Meanwhile I will let Espressif know about lacking the IDF_VER stuff.

A slightly inelegant solution is to not have it depends on anything, and then raise a compilation warning in the code:

Hmm, in this approach we still need to find a way to fix the CI, otherwise it will keep failing. So far I left default n to LITTLEFS_SDMMC_SUPPORT

@BrianPugh I've copied the demo code and implemented something like this:

    auto *sdmmc = sd.get_sdmmc_handle(); // This is our SD card manager returning its internal sdmmc_card_t handle

    esp_vfs_littlefs_conf_t conf = {
            .base_path = "/littlefs",
            .partition_label = nullptr,
            .partition = nullptr,
            .sdcard = sdmmc,
            .format_if_mount_failed = true,
            .read_only = false,
            .dont_mount = false,
            .grow_on_mount = true,
    };

    ESP_ERROR_CHECK(esp_vfs_littlefs_register(&conf));

    ESP_LOGI(TAG, "LittleFS Mounted");


    ESP_LOGI(TAG, "Opening file");
    FILE *f = fopen("/littlefs/hello.txt", "w");
    if (f == NULL)
    {
        ESP_LOGE(TAG, "Failed to open file for writing");
        return;
    }
    fprintf(f, "LittleFS Rocks!\n");
    fclose(f);
    ESP_LOGI(TAG, "File written");

    // Check if destination file exists before renaming
    struct stat st;
    if (stat("/littlefs/foo.txt", &st) == 0)
    {
        // Delete it if it exists
        unlink("/littlefs/foo.txt");
    }

    // Rename original file
    ESP_LOGI(TAG, "Renaming file");
    if (rename("/littlefs/hello.txt", "/littlefs/foo.txt") != 0)
    {
        ESP_LOGE(TAG, "Rename failed");
        return;
    }

    // Open renamed file for reading
    ESP_LOGI(TAG, "Reading file");
    f = fopen("/littlefs/foo.txt", "r");
    if (f == NULL)
    {
        ESP_LOGE(TAG, "Failed to open file for reading");
        return;
    }

    char line[128];
    char *pos;

    fgets(line, sizeof(line), f);
    fclose(f);
    // strip newline
    pos = strchr(line, '\n');
    if (pos)
    {
        *pos = '\0';
    }
    ESP_LOGI(TAG, "Read from file: '%s'", line);

    // All done, unmount partition and disable LittleFS
    esp_vfs_littlefs_unregister_sdmmc(sdmmc);
    ESP_LOGI(TAG, "LittleFS unmounted");

...and it works:

Hi @BrianPugh

In regards to this:

3. Be sure to add unit tests! I've got an esp32 board that has a microsd slot, but I've honestly never used it. Would be good to check your work!

I don't think I can implement this easily, as our company's board is with an uncommon SD NAND chip, similar to the XTSD01GLGEAG chip I mentioned above, and the procedure of creating the sdmmc_card_t requires a separate process which is confidential. I cannot submit that in this PR as it may breach the NDA. Meanwhile currently I also don't have a ESP32 dev board that comes with a SD card slot.

Since this implementation is done and proven working, if you would like to, you can review and approve this PR first. In the meantime I will see if I can buy a dev board and implement the unit test later.

Regards,
Jackson

@BrianPugh somehow I couldn't get the CMake problem fixed as per I said in the comment. But I've put the sdmmc into PRIV_REQUIRES as a workaround instead.

Hi @BrianPugh , may I ask is there any updates on this?

I'm spread a bit thin, but I was able to confirm that no original functionality has been negatively impacted, so I'm happy about that. The code you submitted is also clean and well written. Unfortunately, I dont have the time to personally confirm SD functionality. I don't see any obvious issues with the code, and it appears to be working for you.

So how about this: I merge this PR, and release v1.13.0. Would you be comfortable coming aboard with triage role to give supervision/code-reviews for SD-related issues?

So how about this: I merge this PR, and release v1.13.0. Would you be comfortable coming aboard with triage role to give supervision/code-reviews for SD-related issues?

Yea sure, I'm definitely happy to join!

For our company we are using this functionality for our products as well. So far we can confirm the basic read/write works just fine. If there's any issue, we can fix it (and we have to, as our products also depend on it 😅).

excellent! Tagged v1.13.0 and added you to the project! Thank you very much for this PR and for joining!